Skip to content

Conversation

@odeimaiz
Copy link
Member

@odeimaiz odeimaiz commented Jul 9, 2025

What do these changes do?

In order to get ready for the coming "multiple users access the same study at the same time" feature, this PR centralizes most of the study related calls in the Study Store.

The idea is to have the Study model managed in the Store while it's able to listen to changes pushed from the backend (other users).

Related issue/s

How to test

Dev-ops

@odeimaiz odeimaiz self-assigned this Jul 9, 2025
@odeimaiz odeimaiz added this to the Engage milestone Jul 9, 2025
@odeimaiz odeimaiz added t:enhancement Improvement or request on an existing feature a:frontend issue affecting the front-end (area group) labels Jul 9, 2025
@odeimaiz odeimaiz marked this pull request as ready for review July 10, 2025 07:47
@odeimaiz odeimaiz requested a review from Copilot July 10, 2025 07:47
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR centralizes all study-related API interactions into a singleton osparc.store.Study, replacing ad-hoc osparc.data.Resources.fetch("studies", …) calls in UI components with calls to Study.getInstance(), and adds a suite of new store methods for pagination, trash/untrash, moving, tagging, collaborators, and node operations.

  • Replaced direct resource fetches across many components with osparc.store.Study.getInstance() calls
  • Converted osparc.store.Study to a singleton and added new methods (getPage, trashStudy, moveStudyToWorkspace, addTag, removeCollaborator, etc.)
  • Removed duplicated static methods from the generic Store.js in favor of the specialized Study store

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
services/static-webserver/client/source/class/osparc/study/StudyOptions.js Switched name, wallet and fetch calls to Study.getInstance()
services/static-webserver/client/source/class/osparc/study/NodePricingUnits.js Updated pricing-unit fetch/update to use Study singleton
services/static-webserver/client/source/class/osparc/study/CreateFunction.js Replaced metadata PATCH fetch with store.updateMetadata
services/static-webserver/client/source/class/osparc/study/BillingSettings.js Centralized wallet selection via Study singleton
services/static-webserver/client/source/class/osparc/store/Templates.js Delegated template fetching to Study.getInstance().getOne
services/static-webserver/client/source/class/osparc/store/Study.js Converted to a singleton, added CRUD, pagination, tag, collaborator APIs
services/static-webserver/client/source/class/osparc/store/Store.js Removed study‐specific static methods now in Study store
services/static-webserver/client/source/class/osparc/dashboard/StudyBrowser.js Replaced fetch calls (getPage, getOne, delete, patch) with store calls
services/static-webserver/client/source/class/osparc/dashboard/ResourceBrowserBase.js Moved delete, untrash, patch, pricing workflows into Study singleton
Comments suppressed due to low confidence (2)

services/static-webserver/client/source/class/osparc/store/Study.js:110

  • The updateMetadata method does not return the fetch promise, preventing callers from waiting on or handling errors. Add return before the fetch call to enable proper chaining.
      osparc.data.Resources.fetch("studies", "updateMetadata", params)

services/static-webserver/client/source/class/osparc/store/Study.js:26

  • New pagination methods (getPage, getPageTrashed, getPageSearch) were added without accompanying unit or integration tests. Consider adding tests to ensure correct behavior and catch any regressions.
    getPage: function(params, options) {

Copy link
Contributor

@giancarloromeo giancarloromeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@odeimaiz
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Jul 10, 2025

queue

🟠 Waiting for conditions to match

  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • label=🤖-automerge
      • any of: [🛡 GitHub branch protection]
        • check-neutral = deploy to dockerhub
        • check-skipped = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-neutral = system-tests
        • check-skipped = system-tests
        • check-success = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
        • check-success = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-neutral = integration-tests
        • check-skipped = integration-tests
        • check-success = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
        • check-success = build-test-images (frontend) / build-test-images
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • any of: [🛡 GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed

@sonarqubecloud
Copy link

Please retry analysis of this Pull-Request directly on SonarQube Cloud

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

@odeimaiz
Copy link
Member Author

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Jul 10, 2025

queue

🟠 Waiting for conditions to match

  • -closed [📌 queue requirement]
  • any of: [🔀 queue conditions]
    • all of: [📌 queue conditions of queue default]
      • label=🤖-automerge
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • any of: [🛡 GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [🛡 GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [🛡 GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [🛡 GitHub branch protection]
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of: [📌 queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed

@odeimaiz odeimaiz enabled auto-merge (squash) July 10, 2025 11:33
@sonarqubecloud
Copy link

@odeimaiz odeimaiz merged commit 6acc97b into ITISFoundation:master Jul 10, 2025
59 checks passed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:frontend issue affecting the front-end (area group) t:enhancement Improvement or request on an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants